Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.93%. Comparing base (c01758c) to head (6e015af).
⚠️ Report is 12 commits behind head on main.

@@            Coverage Diff            @@
##               main     #173   +/-   ##
=========================================
  Coverage     64.93%   64.93%           
+ Complexity      981      980    -1     
=========================================
  Files            48       48           
  Lines          3094     3094           
=========================================
  Hits           2009     2009           
  Misses         1085     1085           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I'm not a big fan of this – the kanji mode exists specifically to massively reduce the amount of storage required for that.

Looking at the original zxing code I noticed that they subtract 1 from the length to make it uneven, which we don't do.

Could you test locally if that makes a difference for your result?

I also tried to generate the same QR code with ZXing, which yielded a valid QR code, so there must be some bug in Bacon, but I can't tell, which.

@DASPRiD

I tested your suggestion but unfortunately the result is still unreadable QR code.

I also tried to generate the same QR code with ZXing, which yielded a valid QR code, so there must be some bug in Bacon, but I can't tell, which.

@DASPRiD

Could you test with "あいうえお" not "あいうえお123"?

You can open the link yourself :)

You can open the link yourself :)

My bad😅 (I thought you are testing it in locally.)

It works with "あいうえお" in ZXing. So yeah, I think something wrong with Bacon but I'm not sure what is.

I'd recommend to poke around and compare the code paths for kanji between Bacon and ZXing, it might be some very small knob.

I made an attempt to find the mistake, but haven't succeeded so far.

• Looking at the original zxing code I noticed that they subtract 1 from the length to make it uneven, which we don't do.

That bytes.length - 1 in the zxing code is actually useless:

    int maxI = bytes.length - 1; // bytes.length must be even
    for (int i = 0; i < maxI; i += 2) {

Since we're sure that bytes.length is even and the iterator increments by 2, this loop would perform the same number of iterations as:

    int maxI = bytes.length;
    for (int i = 0; i < maxI; i += 2) {

• Back to the PHP implementation, both iconv() and mb_convert_encoding() provide the expected pairs of bytes:

$bytes = iconv('utf-8', 'SHIFT-JIS', 'あいうえお');
print_r(unpack('C*', $bytes));

$bytes = mb_convert_encoding('あいうえお', 'SHIFT-JIS');
print_r(unpack('C*', $bytes));

Both result in the same expected output:

Array
(
    [1] => 130
    [2] => 160
    [3] => 130
    [4] => 162
    [5] => 130
    [6] => 164
    [7] => 130
    [8] => 166
    [9] => 130
    [10] => 168
)

• So far, I haven't detected errors in the rest of the code, including appendBits() and appendBit().

I managed to get a working QR code by replacing strlen() with mb_strlen() in the following line:

This definitely warrants deeper analysis and understanding, but this is the culprit.

Oh, great find, that does actually make a lot of sense, since the variable literally indicates number of letters, not bytes! Though we are still using iconv in this library, as it's more widely installed, so iconv_strlen would probably be correct here.

Superseded by #200